-
-
Notifications
You must be signed in to change notification settings - Fork 173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
aarch64: support FIQs and use them for TLB shootdown IPIs #1039
aarch64: support FIQs and use them for TLB shootdown IPIs #1039
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dealing with FIQs is kind of annoying. I'm wondering if we should just create a separate/dedicated struct or series of functions for handling FIQs that doesn't go through the same LocalInterruptController API that regular IRQs do.
If there was only GICv3 we could, but as GICv2 uses MMIO for FIQ management I think we should keep this "Use the normal structures but bypass their sync primitives" design: it's pretty simple to understand, unsafe code is very small, and it retains GIC compatibility via the GICv2/GICv3 abstraction. |
Fair enough, I agree. Left a few new comments, should be easy to address pretty quickly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've accepted this PR, thanks! A couple of follow-up changes I'd like to see:
-
InterruptDestination
seems redundant withIpiTargetCpu
...? -
We can use type wrappers to ensure that the
acknowledge_fast_interrupt
andend_of_fast_interrupt
functions are only invokable bycurrent_elx_fiq
. By that I mean create a new typeExceptionContextFiq
that is a simple wrapper aroundExceptionContext
, and then require a reference to that type to be passed into both of the above fast interrupt functions. Then, we can make those functions safe on the outside, and just use unsafe on the inside.#[transparent] #[non_exhaustive] pub struct ExceptionContextFiq(ExceptionContext);
This design is better imo since there's no real way to ensure that the fast interrupt functions are actually called safely.
-
It's a little bit odd to use the type
*const InterruptHandler
, sinceInterruptHandler
is already a function pointer. We should be able to remove the leading*const
part and just deal with theInterruptHandler
directly. But do let me know if that is actually requried somewhere else.
* Add support for fast interrupts on aarch64, aka FIQs. FIQs are designed to be fast and can thus interrupt regular interrupts (IRQs) that are in the process of being handled. They are similar to NMIs on x86_64 in this regard, but can also be explicitly enabled/disabled. * Updated the GIC driver to support both Group 0 (FIQs) and Group 1 (IRQs). * `nano_core`, `captain`, and `ap_start` now enable/disable FIQs. * Broadcasting TLB shootdown IPIs now uses FIQs to ensure that TLB shootdowns occur instantly even if regular interrupts are disabled on one or more other CPUs. * Add a separate trait `Aarch64LocalInterruptController` for arch-specific features, which keeps the `LocalInterruptController` arch-agnostic. * This trait is primarily for configuring/handling fast interrupts (FIQs), but also for acknowledging interrupts, which x86_64 does not require. * The interrupt controller now allows enabling/disabling SPIs too. --------- Co-authored-by: Kevin Boos <kevinaboos@gmail.com> ac51fc1
…#1039) * Add support for fast interrupts on aarch64, aka FIQs. FIQs are designed to be fast and can thus interrupt regular interrupts (IRQs) that are in the process of being handled. They are similar to NMIs on x86_64 in this regard, but can also be explicitly enabled/disabled. * Updated the GIC driver to support both Group 0 (FIQs) and Group 1 (IRQs). * `nano_core`, `captain`, and `ap_start` now enable/disable FIQs. * Broadcasting TLB shootdown IPIs now uses FIQs to ensure that TLB shootdowns occur instantly even if regular interrupts are disabled on one or more other CPUs. * Add a separate trait `Aarch64LocalInterruptController` for arch-specific features, which keeps the `LocalInterruptController` arch-agnostic. * This trait is primarily for configuring/handling fast interrupts (FIQs), but also for acknowledging interrupts, which x86_64 does not require. * The interrupt controller now allows enabling/disabling SPIs too. --------- Co-authored-by: Kevin Boos <kevinaboos@gmail.com> ac51fc1
Sometimes when we tried spawning an interactive console in theseus on aarch64, by sending keystrokes through the UART port, theseus hanged.
We then found out that a complicated scenario was taking place:
This brings the following changes:
interrupts
crate was modified to handle FIQs.This depend on a PR in irq_safety, so it doesn't build at this moment.